Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pull up setDisableMBeanRegistry to ConfigurableTomcatWebServerFactory #44071

Closed
wants to merge 1 commit into from
Closed

Pull up setDisableMBeanRegistry to ConfigurableTomcatWebServerFactory #44071

wants to merge 1 commit into from
+13 −16

Conversation

panic08
Copy link
Contributor

@panic08 panic08 commented Feb 4, 2025 •
edited
Loading

Since all inheritors implement setDisableMBeanRegistry we should put it in their interface

Fix #44070

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 4, 2025
@panic08 panic08 force-pushed the pull-up-setDisableMBeanRegistry-to-ConfigurableTomcatWebServerFactory branch from 411a1f3 to afd463e Compare February 4, 2025 17:56
@panic08
Copy link
Contributor Author

panic08 commented Feb 4, 2025 •
edited
Loading

However, I have a reasonable question for me. You may notice that I removed @since in the above method in javadoc. Should we keep it? Ideally we have just moved it as a new method to the interface, but on the other hand the method was in the inheritors before.

@philwebb
Copy link
Member

philwebb commented Feb 4, 2025

Thanks for the pull-request @panic08. Having looked at this in more detail, we're going to need to do a little more design work and push this back to 4.0.x. The goal behind pulling up the method was to refactor TomcatReactiveWebServerFactoryCustomizer and TomcatServletWebServerFactoryCustomizer, but that turns out to be quite hard to do without breaking things. Thanks anyway for your efforts.

@philwebb philwebb closed this Feb 4, 2025
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pull up setDisableMBeanRegistry to ConfigurableTomcatWebServerFactory
3 participants
@panic08 @philwebb @spring-projects-issues